Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APS-1677 AP occupancy view #2264

Merged
merged 1 commit into from
Dec 24, 2024
Merged

Conversation

bobmeredith
Copy link
Contributor

@bobmeredith bobmeredith commented Dec 20, 2024

Context

https://dsdmoj.atlassian.net/browse/APS-1677

Changes in this PR

Adds the occupancy view (calendar) page with a link from the premises view action menu.

Screenshots of UI changes

image

image

@bobmeredith bobmeredith force-pushed the feature/APS-1677_ap_occupancy_view branch from 5dc8ed4 to 5e0bcce Compare December 20, 2024 13:34
<ul class="calendar__month govuk-list">
{% for day in month.days %}
<li class="calendar__day calendar__day--{{ day.name.slice(0,3) | lower }}{% if day.status !== 'available' %} {{ 'govuk-tag--yellow' if day.status in ['availableForCriteria','full'] else 'govuk-tag--red' }}{% endif %}">
<a class="calendar__link" href="{% if day.link %}{{ day.link }}{% else %}#{% endif %}">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this assumes that all calendars are rendered with a link property in their day structures. I could put this in the lower level day templates, but that would mean more of the day rendering, including the date name, would be duplicated.

@bobmeredith bobmeredith marked this pull request as ready for review December 20, 2024 18:04
Copy link
Contributor

@aliuk2012 aliuk2012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me and seems sensible.

Only other nit picking suggestion I could make is to maybe squash the commits?

@@ -21,5 +21,13 @@ export const premisesActions = (user: UserDetails, premises: Premises) => {
})
}

if (user.permissions.includes('cas1_premises_view') && premises.supportsSpaceBookings) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to an explicit test for permission - but this permission is already given to users in prod.
The supportsSpaceBookings flag however is not set on any premises ATM, and will be set on rollout in February.

@bobmeredith bobmeredith force-pushed the feature/APS-1677_ap_occupancy_view branch from df4809f to 7c0a7ee Compare December 23, 2024 13:00
@bobmeredith bobmeredith force-pushed the feature/APS-1677_ap_occupancy_view branch from db3c4fd to d27d30f Compare December 24, 2024 10:58
@bobmeredith bobmeredith merged commit 639e8cc into main Dec 24, 2024
7 checks passed
@bobmeredith bobmeredith deleted the feature/APS-1677_ap_occupancy_view branch December 24, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants